Conversation
vsh123
requested changes
Feb 19, 2022
vsh123
left a comment
There was a problem hiding this comment.
윤경님 안녕하세요!
1단계 미션 잘 구현해주셨네요 :)
몇가지 코멘트를 남겼으니 확인부탁드리겠습니다~
| # java-blackjack No newline at end of file | ||
| # java-blackjack | ||
|
|
||
| ## 연료 주입 기능 목록 |
|
|
||
| @Override | ||
| public double getChargeQuantity() { | ||
| return getTripDistance() / getDistancePerLiter(); |
There was a problem hiding this comment.
getTripDistance()대신 상태를 바로 호출하면 어떨까요?
Suggested change
| return getTripDistance() / getDistancePerLiter(); | |
| return this.distance / getDistancePerLiter(); |
|
|
||
| public static final int FIRST_INDEX = 0; | ||
|
|
||
| private List<Card> cards; |
Comment on lines
+22
to
+24
| Arrays.stream(Denomination.values()) | ||
| .map(denomination -> new Card(shape, denomination)) | ||
| .forEach(cards::add); |
There was a problem hiding this comment.
Denomination.values를 stream으로 돌리긴 했지만 결국 depth가 2를 넘는 것과 같아보이는데요! 조금 더 작은 단위의 메서드로 분리해보면 어떨까요?
| return cards; | ||
| } | ||
|
|
||
| public Card pickOneCard() { |
| this.value = value; | ||
| } | ||
|
|
||
| public String getSign() { |
| public Player(String name, List<Card> cards) { | ||
| this.name = name; | ||
| this.cards.addAll(cards); | ||
| this.sum = getSumOfCards(); //TODO |
There was a problem hiding this comment.
TODO는 아직 미완성일걸까요?- sum이
canHit에서만 사용되는데,getSumOfCards에도 사용되어야하지 않을까요?!
| return sum <= PLAYER_MAXIMUM_SUM; | ||
| } | ||
|
|
||
| public int getSumOfCards() { |
There was a problem hiding this comment.
블랙잭의 규칙에 따르면 ACE는 1점과 11점모두 가능합니다!
만약 KING과 ACE라면 점수는 21점이 되어야하는데, 현재는 11점이되는 것 같아요! 요고 한번만 확인해주세요~
아래는 테스트입니다.
@Test
void ACE는_1과_11점_두가지_모두_가능하다() {
//given
Player player = new Player("kim", Arrays.asList(
new Card(Shape.SPADE, Denomination.KING),
new Card(Shape.SPADE, Denomination.ACE)
));
//when
int actual = player.getSumOfCards();
//then
assertThat(actual).isEqualTo(21);
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
안녕하세요. 리뷰어님!
이번 미션은 해림님(@Hae-Riri)과 진행했습니다.
페어님의 도움을 많이 받았는데, 부족한 부분 계속 보완하겠습니다.